refactor(compilation): extract ccache into a swappable extension#122
refactor(compilation): extract ccache into a swappable extension#122iav wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59c1420d7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
59c1420 to
29a4bb9
Compare
|
@chatgpt review Sorry for the wrong mention earlier — please re-verify on head 29a4bb9. Variant B rewrite addresses both prior P2 findings:
Four open architectural questions are in the PR description above. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29a4bb93be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Commit 7 (head P1 'auto-enable after remote': shim in main-config.sh now also triggers on P2 'keep legacy ccache helpers': P2 'export private CCACHE_DIR' / 'export shared-cache umask': P2 'preserve kernel make PATH prefix': P2 'defer private cache setup until remote config runs': All env wiring (CCACHE_DIR, CCACHE_UMASK, PATH, ${CCACHE}) moved out of Ordering invariant is now documented in the file header. |
|
@codex review (Correcting the mention handle — |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05bf8ee571
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Commit 8 (head Now:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2185ea647c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4a7cdb673
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
f4f17e7 to
f32ac58
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f32ac58965
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
f9a751c to
d3170e8
Compare
The variable has been declared but never assigned or used since 8278dc5 (2023-06-20, 'allwinner: Enable crust compilation'), which copy-pasted the binutils_version + binutils_flags skeleton from atf.sh but did not bring along the gcc/ld probe block that conditionally populates the flag, nor the TF_LDFLAGS-style use site. Crust is or1k-elf firmware where --no-warn-rwx-segment is not relevant, so the absence of the probe is by design; the leftover empty declaration is just dead. Removes a pre-existing SC2034 'appears unused' warning that surfaced when the file is touched by other refactors. Assisted-by: Claude:claude-opus-4.7
CROSS_COMPILE and CC were hard-coded to 'ccache' which:
- breaks when USE_CCACHE=no (CCACHE='' but 'ccache' still tried)
- prevents wrapper substitution by extensions (sccache, distcc)
Match the established kernel/uboot convention where the cache wrapper
is parameterised via ${CCACHE} (set by configure-ccache based on
USE_CCACHE). Default value '' means no wrapper.
Assisted-by: Claude:claude-opus-4.7
…le_wrapper
Adds three new extension points to the compilation pipeline:
- atf_make_config: called right before invoking 'make' for ATF (TF-A).
Receives env that will be passed to make; extensions can mutate
CCACHE / CC / CROSS_COMPILE to inject wrappers.
- crust_make_config: same for the or1k Crust firmware (defconfig +
target make).
- do_with_compile_wrapper / compile_wrapper_pre / compile_wrapper_post:
high-level wrapper that bookends an arbitrary compilation block
with pre/post hooks. Lets extensions observe a compile pass
end-to-end (clear stats / dump stats / mount caches / etc) without
having to patch each artifact's compile.sh.
Pure addition: no existing call site is wired yet (kernel/uboot
migration follows in subsequent commits). The hooks are no-ops until
an extension implements them.
Assisted-by: Claude:claude-opus-4.7
Extracts ccache-specific logic from lib/functions/compilation/ccache.sh
into a new opt-out extension extensions/ccache.sh. The extension wires
itself into the compilation pipeline through the new hook points:
- compile_prepare_vars: dispatched from prepare_compilation_vars
(early). Computes CCACHE / CCACHE_DIR / CCACHE_UMASK / PATH and
exports them so subsequent make-quoted-params arrays expand
correctly. Runs before any artifact's make invocation.
- compile_wrapper_pre/_post: zeros / reports per-build stats around
each artifact's compile pass.
- {kernel,uboot,atf,crust}_make_config: ensures CCACHE_DIR /
CCACHE_UMASK are visible to the make subshell on each artifact.
This is an additive change: nothing yet calls call_extension_method
"compile_prepare_vars" — that wiring lands in the next refactor commit.
Assisted-by: Claude:claude-opus-4.7
…che.sh
Wires the new ccache extension (extensions/ccache.sh) into the
compilation pipeline, replacing the in-core lib/functions/compilation/
ccache.sh module that was sourced unconditionally.
Changes:
- lib/functions/compilation/kernel.sh, uboot.sh: wrap the compile
pass with do_with_compile_wrapper so the new compile_wrapper_pre/
_post hooks fire around it.
- lib/functions/configuration/compilation-config.sh: dispatch
compile_prepare_vars (early phase, before any make-quoted-params
array is built) instead of inlining configure-ccache. Also clears
stale CCACHE export from a previous artifact iteration so a
second uboot/kernel call in the same shell can't reuse a now-
disabled cache. Keeps a one-shot deprecation warning if any
user-patch still sources the legacy lib.config.
- lib/functions/configuration/main-config.sh: if USE_CCACHE=yes
(or the ccache-remote extension is enabled) auto-enable the
'ccache' extension before initialize_extension_manager runs.
Honours both EXT alias and the legacy space-separated
ENABLE_EXTENSIONS form.
- lib/functions/general/extensions.sh: extract
extension_list_normalized helper used by the auto-enable shim.
- lib/functions/compilation/ccache.sh: removed (logic moved to
extensions/ccache.sh).
Behaviour is preserved for USE_CCACHE=yes (default-on for kernel/uboot
builds) and USE_CCACHE=no callers. ENABLE_EXTENSIONS=ccache-remote
implicitly pulls in ccache, matching pre-refactor coupling.
Assisted-by: Claude:claude-opus-4.7
d3170e8 to
6dd6a7c
Compare
Goal
Move ccache out of core compilation into a swappable extension. After this PR a parallel cache backend (sccache, distcc-style, distributed cache) can ship as a sibling extension without touching core.
A concrete near-term motivation: a future
extensions/sccache.shwould let CI workflows running on GitHub-hosted runners route their compile-cache straight into the native GitHub Actions cache (SCCACHE_GHA_ENABLED=on, viamozilla-actions/sccache-action) — 10 GB free per repository, low-latency on the runner's local region. The same extension framework also covers self-hosted runners (S3/R2/MinIO bucket, WebDAV endpoint, Redis-on-disk, ...) without core changes — backend choice is purely an env-var concern per workflow.What changes (vs
armbian/build:main)New files
extensions/ccache.sh— implementscompile_prepare_vars,compile_wrapper_pre/_post, and{kernel,uboot,atf,crust}_make_confighooks.lib/functions/compilation/compile-wrapper.sh—do_with_compile_wrapper+compile_wrapper_pre/_posthooks.Removed
lib/functions/compilation/ccache.sh(logic moved into the extension).Modified
lib/functions/compilation/atf.sh— addsatf_make_confighook; replaces hardcodedccachewith${CCACHE}(latent bug:USE_CCACHE=nowas still draggingccacheinto ATF).lib/functions/compilation/crust.sh— addscrust_make_confighook; drops deadbinutils_flags_crust=""declaration (preexisting SC2034 since 2023).lib/functions/compilation/{kernel,uboot}.sh— compile phase wrapped indo_with_compile_wrapper.lib/functions/configuration/compilation-config.sh— dispatchescompile_prepare_varsinstead of inliningconfigure-ccache; resets staleCCACHEbetween artifacts; warns ifUSE_CCACHEis set inuserpatches/lib.config(sourced too late forenable_extension).lib/functions/configuration/main-config.sh— auto-enables theccacheextension whenUSE_CCACHE=yes,PRIVATE_CCACHE=yes, orENABLE_EXTENSIONS=ccache-remote, beforeinitialize_extension_manager.lib/functions/general/extensions.sh— addsextension_list_normalizedhelper (used by the auto-enable shim and per-extension mutex checks).lib/library-functions.sh— regenerated.User-visible behavior
Unchanged.
USE_CCACHE=yes(default for kernel/u-boot) andUSE_CCACHE=nobehave exactly as before;ENABLE_EXTENSIONS=ccache-remoteimplicitly pulls inccache, matching the pre-refactor coupling.Test plan
Built on a fresh DigitalOcean cloud builder (AMD EPYC 8c, Ubuntu 24.04),
BOARD=helios64 BRANCH=edge:USE_CCACHE=yeskernel coldCcache result hit=12 miss=14410USE_CCACHE=nokernelUSE_CCACHE=yesuboot+ATF coldCcache result hit=8 miss=1002ENABLE_EXTENSIONS=ccache-remote+CCACHE_REMOTE_STORAGE=http://m1:8088/ccache/over WireGuardRemote ccache result hit=4536 miss=2791 write=2740 err=0 (61%)Logs: paste.armbian.com/{enaretezuc,noxayoyoni,vonuyajido}.
Known design choices
USE_CCACHE=yesinuserpatches/lib.configdoes not auto-enable theccacheextension.lib.configis sourced afterinitialize_extension_manager, too late to enable extensions; theframework limitation is documented in
main-config.sh(too late to define hook functions or add extensions in lib.config). The shimemits a warning at compile-vars phase nudging users to migrate to
ENABLE_EXTENSIONS=ccache(the auto-enable code path).USE_CCACHE=yes,PRIVATE_CCACHE=yes, orENABLE_EXTENSIONS=ccache-remote(the onlyextension in-tree that sets
USE_CCACHE=yesfrom itsextension_prepare_config). Any future extension that also setsUSE_CCACHEfrom its prepare hook will need to add itself to thesame trigger list, since the shim runs before
initialize_extension_manager.Summary by CodeRabbit
New Features
Refactor